Skip to content

Best effort to decode nester error strings from reverts#184

Open
davecrighton wants to merge 4 commits intohyperledger:mainfrom
davecrighton:djc/nestedErrorDecode
Open

Best effort to decode nester error strings from reverts#184
davecrighton wants to merge 4 commits intohyperledger:mainfrom
davecrighton:djc/nestedErrorDecode

Conversation

@davecrighton
Copy link
Copy Markdown

Additional change related to hyperledger/firefly#1717

This provides a best-effort case for decoding error strings from reverts where the revert message has been constructed by nesting customer error types or generic errors (see the linked issue for details).

The approach taken is:

After decoding the outer Error(string) we scan forwards in the resulting string finding the first occurence of either any known error selector provided in the ABI list or the generic error selector.

If a selector is found the remainder of the string is decoded using either standard ABI decoding for generic errors or the existing formatCustomError function for custom errors.

If no selector matches, or if decoing fails any remaining bytes are converted to a hex encoded string.

I did consider possibly additionally stripping null bytes to see if we could return a readable string in more cases but I considered it might be more confusing to get a halfway approach rather than representing an entire nested error as either always decoded or always hex.

@davecrighton davecrighton force-pushed the djc/nestedErrorDecode branch from 1de5d77 to 1e98a85 Compare March 2, 2026 11:27
@davecrighton davecrighton marked this pull request as ready for review March 2, 2026 12:33
@davecrighton davecrighton requested a review from a team as a code owner March 2, 2026 12:33
Copy link
Copy Markdown
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first pass - @davecrighton can you sign off the commits for DCO please

Copy link
Copy Markdown
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To aid a review @davecrighton - please could you provide in the subject:

  • A Solidity example of the coding pattern that results in the problem
  • A link to somewhere that helps us understand how standard this is
    • e.g. is it a link to the Solidity docs, or OpenZeppelin, or some forums on coding suggestions
  • An description of what the algorithm is we're applying
    • I understand from your comment this only works for Error(string) "default" revert errors, but what are we inferring about the string placed in there and why

Copy link
Copy Markdown
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second request from me:

Can we make this a pkg utility function (honestly I think firefly-signer would be the ideal place, but we've started putting package utilities in firefly-evmconnect too a little recently).

This code feels important to standardize as DecodeRevertErrorBytes() and return a well structured struct return that is documented, that has a String() function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants